Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Implement proper SSSOM validation #32

Merged
merged 8 commits into from
Nov 13, 2023
Merged

Implement proper SSSOM validation #32

merged 8 commits into from
Nov 13, 2023

Conversation

matentzn
Copy link
Member

@matentzn matentzn commented Nov 13, 2023 •
edited
Loading

Fixes #30 and #31

@matentzn matentzn requested a review from glass-ships November 13, 2023 09:40
@matentzn matentzn mentioned this pull request Nov 13, 2023
Closed
@glass-ships
Copy link
Contributor

should we also add metadata for mondo?
it seems to already be in the header, but:

    curie_map:
      BFO: http://purl.obolibrary.org/obo/BFO_
      CSP: http://purl.obolibrary.org/obo/mondo/mappings/unknown_prefix/CSP/
      DECIPHER: http://purl.obolibrary.org/obo/mondo/mappings/unknown_prefix/DECIPHER/
      DERMO: http://purl.obolibrary.org/obo/mondo/mappings/unknown_prefix/DERMO/
      DOID: http://purl.obolibrary.org/obo/DOID_
      EFO: http://www.ebi.ac.uk/efo/EFO_
      GARD: http://purl.obolibrary.org/obo/mondo/mappings/unknown_prefix/GARD/
      GTR: http://purl.obolibrary.org/obo/mondo/mappings/unknown_prefix/GTR/
      HGNC: http://identifiers.org/hgnc/
      HP: http://purl.obolibrary.org/obo/HP_
      ICD10CM: http://purl.bioontology.org/ontology/ICD10CM/
      ICD10EXP: http://purl.obolibrary.org/obo/mondo/mappings/unknown_prefix/ICD10EXP/
      ICD10WHO: https://icd.who.int/browse10/2019/en#/
      ICD9: http://purl.obolibrary.org/obo/mondo/mappings/unknown_prefix/ICD9/
      ICD9CM: http://purl.obolibrary.org/obo/mondo/mappings/unknown_prefix/ICD9CM/
      ICDO: http://purl.obolibrary.org/obo/mondo/mappings/unknown_prefix/ICDO/
      IDO: http://purl.obolibrary.org/obo/IDO_
      IEDB: http://purl.obolibrary.org/obo/mondo/mappings/unknown_prefix/IEDB/
      LOINC: http://purl.obolibrary.org/obo/mondo/mappings/unknown_prefix/LOINC/
      MEDGEN: http://identifiers.org/medgen/
      MESH: http://identifiers.org/mesh/
      MFOMD: http://purl.obolibrary.org/obo/MFOMD_
      MONDO: http://purl.obolibrary.org/obo/MONDO_
      MP: http://purl.obolibrary.org/obo/MP_
      MPATH: http://purl.obolibrary.org/obo/MPATH_
      MTH: http://purl.obolibrary.org/obo/mondo/mappings/unknown_prefix/MTH/
      MedDRA: http://identifiers.org/meddra/
      NCIT: http://purl.obolibrary.org/obo/NCIT_
      NDFRT: http://purl.obolibrary.org/obo/mondo/mappings/unknown_prefix/NDFRT/
      NIFSTD: http://purl.obolibrary.org/obo/mondo/mappings/unknown_prefix/NIFSTD/
      OBI: http://purl.obolibrary.org/obo/OBI_
      OGMS: http://purl.obolibrary.org/obo/OGMS_
      OMIM: https://omim.org/entry/
      OMIMPS: https://omim.org/phenotypicSeries/PS
      OMOP: http://purl.obolibrary.org/obo/mondo/mappings/unknown_prefix/OMOP/
      ONCOTREE: http://purl.obolibrary.org/obo/mondo/mappings/unknown_prefix/ONCOTREE/
      Orphanet: http://www.orpha.net/ORDO/Orphanet_
      PATO: http://purl.obolibrary.org/obo/PATO_
      PMID: http://purl.obolibrary.org/obo/mondo/mappings/unknown_prefix/PMID/
      RO: http://purl.obolibrary.org/obo/RO_
      Reactome: http://purl.obolibrary.org/obo/mondo/mappings/unknown_prefix/Reactome/
      SCDO: http://purl.obolibrary.org/obo/SCDO_
      SCTID: http://identifiers.org/snomedct/
      UMLS: http://linkedlifedata.com/resource/umls/id/
      Wikidata: http://purl.obolibrary.org/obo/mondo/mappings/unknown_prefix/Wikidata/
      Wikipedia: http://purl.obolibrary.org/obo/mondo/mappings/unknown_prefix/Wikipedia/
      oboInOwl: http://www.geneontology.org/formats/oboInOwl#
      orphanet.ordo: http://www.orpha.net/ORDO/Orphanet_
      owl: http://www.w3.org/2002/07/owl#
      rdf: http://www.w3.org/1999/02/22-rdf-syntax-ns#
      rdfs: http://www.w3.org/2000/01/rdf-schema#
      semapv: https://w3id.org/semapv/vocab/
      skos: http://www.w3.org/2004/02/skos/core#
      sssom: https://w3id.org/sssom/

@matentzn
Copy link
Member Author

You mean because of the "unknowns"? In this case, make a MONDO issue about it - they should fix it!

@glass-ships
Copy link
Contributor

ah. no, i didn't even notice the "unknowns". I just meant that #30 seemed to want curie_map added for all mapping sets listed in the registry, but this PR only did so for biomappings and gene mappings

@matentzn
Copy link
Member Author

Mondo already has it, right, so no need to add another one?

@glass-ships glass-ships merged commit e9582ff into main Nov 13, 2023
2 checks passed
@glass-ships glass-ships deleted the sssom-mappings branch November 13, 2023 21:04
@matentzn
Copy link
Member Author

You have merged this but qc was failing! (Because sssom py cannot process gene_mappings). In any case @hrshdhgd is trying profile the validation run to figure out
If this is fixable.

@glass-ships glass-ships restored the sssom-mappings branch November 13, 2023 21:09
@glass-ships glass-ships mentioned this pull request Nov 13, 2023
Merged
@glass-ships
Copy link
Contributor

reverted

@matentzn
Copy link
Member Author

It's fine to merge the PR, in any case nothing was more broken afterwards then it was before.. it was just a general warning not to merge stuff if qc fails

@matentzn
Copy link
Member Author

Are you making a new PR?

@glass-ships
Copy link
Contributor

as of this commit, f589a59 the tests were passing, at least as far as i can tell?

@glass-ships
Copy link
Contributor

glass-ships commented Nov 13, 2023 •
edited
Loading

Are you making a new PR?

Trying to, but git doesn't seem to think there's any changes between this branch and main anymore, so I'm not sure.

@matentzn
Copy link
Member Author

Huh whaaaa?? Sorry man. Seems all good then! I didn't get a message it's all fixed!

@matentzn
Copy link
Member Author

You may have to revert the revert

@hrshdhgd
Copy link
Collaborator

The QC passes but it still took 50 mins to pass. sssom was not added as a dependency in the toml file and that's why it was failing in the first place. I don't know why it was thinking for over an hour.

@matentzn
Copy link
Member Author

No it definetly had sssom installed, as it finished the pipeline parts before that - but maybe an older version?

@hrshdhgd
Copy link
Collaborator

I set up locally from scratch and sssom was not in the toml file. I added it and that led to qc passing.

@hrshdhgd
Copy link
Collaborator

Don't revert anything. I will make a new PR if need be.

@matentzn
Copy link
Member Author

Please don't lose all my changes 😅 thanks

@hrshdhgd
Copy link
Collaborator

haha don't worry. Nothing will be lost.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@glass-ships glass-ships glass-ships approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add curie_map to the mapping_sets listed in the registry.yml
3 participants
@matentzn @glass-ships @hrshdhgd